refactor: [M3-6300] - MUI v5 Migration - Components > CheckoutBar
#9051
Conversation
Passing run #3186 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
|||||||||||||||
| > | ||
| {details} | ||
| </Typography> | ||
| )} | ||
| </div> | ||
| ) : null} |
There was a problem hiding this comment.
details is typed as a string or number, so I went with the ternary here.
| }, | ||
| checkoutSection: { | ||
| animation: '$fadeIn 225ms linear forwards', | ||
| opacity: 0, |
There was a problem hiding this comment.
It wasn't clear to me if animation and opacity were actually doing anything here, or what the benefit of keeping them would be, so I removed those properties.
There was a problem hiding this comment.
Not totally sure either, but it seems like the "Create Cluster" button does a slight fade in when the component first loads in prod. I don't think this is necessary and initial rendering + interacting with the CheckoutBar looks smooth to me without any animation
There was a problem hiding this comment.
This looked good when testing with the preview link. Can we add a changelog entry?
I noticed with local development (this branch and on develop), we don't display the HA section for kube checkout even though showHighAvailability is true. Do you know why that is?
packages/manager/src/components/CheckoutBar/DisplaySectionList.tsx
Outdated
Show resolved
Hide resolved
| }, | ||
| checkoutSection: { | ||
| animation: '$fadeIn 225ms linear forwards', | ||
| opacity: 0, |
There was a problem hiding this comment.
Not totally sure either, but it seems like the "Create Cluster" button does a slight fade in when the component first loads in prod. I don't think this is necessary and initial rendering + interacting with the CheckoutBar looks smooth to me without any animation
It displays on mine; |
| root: { | ||
| })); | ||
|
|
||
| const StyledDiv1 = styled('div')(() => { |
There was a problem hiding this comment.
Do we have a convention for descriptively naming these styled components? For example, Root and CheckoutSection (the old JSS classnames) could be used instead of StyledDiv1 and StyledDiv2 and might make these names more meaningful.
There was a problem hiding this comment.
Thanks for asking this @hkhalil-akamai -- I think this is a good point and don't love StyledDiv1 or StyledDiv2 either. I do think it's useful to include the element name in the styled component name. I wonder if something like RootStyledDiv and CheckoutSectionStyledDiv would help address both of these points?
There was a problem hiding this comment.
Agreed or StyledRoot and StyledCheckoutSection... just something more descriptive 👍
There was a problem hiding this comment.
REACT_APP_LKE_HIGH_AVAILABILITY_PRICEprobably isn't defined in your.envfile
Yep, that'll do it!
Pending some decision on the discussion about styled component naming (maybe this is a potential cafe topic?), the component in the UI looks good, so I'm approving.
dwiley-akamai
force-pushed
the
M3-6300-tss-reactify-checkoutbar
branch
from
84a8aa5 to
278b128
Compare
Description 📝
Migrates
SRC > Components > CheckoutBarfrom JSS to styled components.Major Changes 🔄
packages/manager/src/components/CheckoutBar/index.tsCheckoutBar.tsx, housed in/CheckoutBar/styles.tsHow to test 🧪
Go to
/kubernetes/createand ensure the checkout bar looks and functions the same as in prod.To-Do:
sxrefactoring